tests/languages/unity/Tests.cs (1)

56-75: Eliminate flakiness: replace fixed delays with a completion signal + timeout

Waiting a fixed 5s can be flaky and slow. Use a TaskCompletionSource signaled by the callback and race it with a timeout.

-            string realtimeResponse = "No realtime message received within timeout";            
-            RealtimeSubscription subscription = null;
-            subscription = realtime.Subscribe(new [] { "tests" }, (eventData) => 
+            var tcs = new TaskCompletionSource<string>();
+            RealtimeSubscription subscription = null;
+            subscription = realtime.Subscribe(new [] { "tests" }, (eventData) =>
             {
                 Debug.Log($"[Test] Realtime callback invoked! Payload count: {eventData.Payload?.Count}");
                 if (eventData.Payload != null && eventData.Payload.TryGetValue("response", out var value))
                 {
                     Debug.Log($"[Test] Found response value: {value}");
-                    realtimeResponse = value.ToString();
-                    Debug.Log($"[Test] Updated realtimeResponse to: {realtimeResponse}");
+                    tcs.TrySetResult(value.ToString());
                 }
                 else
                 {
                     Debug.Log("[Test] No 'response' key found in payload");
                 }
                 subscription?.Close();
             });
-
-            await Task.Delay(5000);
+            var completed = await Task.WhenAny(tcs.Task, Task.Delay(TimeSpan.FromSeconds(10)));
+            var realtimeResponse = completed == tcs.Task ? tcs.Task.Result : "No realtime message received within timeout";
@@
-            await Task.Delay(5000);
-            Debug.Log(realtimeResponse);
+            Debug.Log(realtimeResponse);

Also applies to: 185-189

templates/unity/LICENSE.twig (1)

1-1: License rendering is fine; consider avoiding escaping.

If the generator enables auto-escaping for text templates, render raw to preserve formatting.

-{{sdk.license}}
+{{ sdk.license|default('')|raw }}

templates/unity/ProjectSettings/ProjectVersion.txt (1)

1-2: Parameterize Unity Editor version to reduce maintenance.

Hardcoding the exact patch ties the generator to a single image/tag. Make this a Twig template and feed from config, with sensible defaults.

-m_EditorVersion: 2021.3.45f1
-m_EditorVersionWithRevision: 2021.3.45f1 (3409e2af086f)
+m_EditorVersion: {{ unity.editorVersion|default('2021.3.45f1') }}
+m_EditorVersionWithRevision: {{ unity.editorVersionWithRevision|default('2021.3.45f1 (3409e2af086f)') }}

Also consider targeting a current LTS in CI while still allowing generation for older versions via config.

templates/unity/Assets/Runtime/Core/csc.rsp (1)

1-1: Confirm csc.rsp pickup scope and add trailing newline.

Unity picks up csc.rsp files under Assets, but placement nuances exist across versions. Please verify this file is detected under Assets/Runtime/Core for 2021.3; if not, move a single csc.rsp to Assets/ to apply project-wide. Add a newline at EOF to avoid diff churn.

.github/workflows/tests.yml (1)

53-53: Matrix entry name implies version-lock; consider generalizing.

If you’ll test multiple Unity LTS versions over time, consider “Unity” with a separate matrix dimension for version, or generate from a variable. This reduces churn across files.

templates/unity/Assets/Runtime/Core/Appwrite.Core.asmdef.twig (1)

4-6: UniTask reference vs optional dependency. Decide one path and align.

Referencing “UniTask” here hard-requires the package; the versionDefines block implies it should be optional. Either:

• Keep the reference and declare the dependency in your Unity package (package.json) so consumers get UniTask automatically, or
• Make UniTask truly optional: remove the reference and guard all usages behind #if UNI_TASK with no public API surface exposing UniTask types when absent.

If choosing optional, apply:

-    "references": [
-        "UniTask"
-    ],
+    "references": [],

If choosing required, add UniTask to package.json “dependencies” (see comment in package.json.twig).

templates/unity/Assets/Editor/Appwrite.Editor.asmdef.twig (1)

2-7: Set rootNamespace to the Editor namespace for consistency.

Editor assemblies typically use a distinct namespace to avoid collisions with runtime types.

-    "name": "{{ spec.title | caseUcfirst }}.Editor",
-    "rootNamespace": "{{ spec.title | caseUcfirst }}",
+    "name": "{{ spec.title | caseUcfirst }}.Editor",
+    "rootNamespace": "{{ spec.title | caseUcfirst }}.Editor",

templates/unity/package.json.twig (2)

2-7: Package identity should be vendor-agnostic and configurable.

Hardcoding com.fellmonkey isn’t ideal for the generator. Expose a vendor/namespace input and default to org-owned value.

-  "name": "com.fellmonkey.{{spec.title | caseLower}}-sdk",
+  "name": "{{ sdk.packageName|default('com.appwrite.' ~ (spec.title | caseLower) ~ '-sdk') }}",
   "version": "{{sdk.version}}",
   "displayName": "{{spec.title}} SDK",
   "description": "{{spec.description}}",
-  "unity": "2021.3",
+  "unity": "{{ sdk.unityMinVersion|default('2021.3') }}",

---

8-17: Add runtime UPM dependencies to templates/unity/package.json.twig

Insert a "dependencies" block immediately after the "keywords" array in templates/unity/package.json.twig declaring com.cysharp.unitask and com.endel.nativewebsocket (use the same UPM URLs/versions from templates/unity/Packages/manifest.json) so consumers auto-install required packages.

templates/unity/Packages/packages-lock.json (1)

1-490: Reconsider shipping a lockfile in a generator template.

packages-lock.json tightly pins the dependency graph to Unity 2021.3 and specific package states. For a cross-version template, this can cause resolution failures or needless churn. Prefer:

• Omit this file from the template (let UPM resolve from manifest), or
• Generate it conditionally per targeted Unity editor in CI and don’t commit it.

templates/unity/Packages/manifest.json (2)

5-15: Trim dev-only/editor convenience packages from a library template.

Consider removing non-essential deps to reduce install time and conflicts:

• com.unity.collab-proxy
• com.unity.ide.rider / com.unity.ide.visualstudio / com.unity.ide.vscode
• com.unity.feature.2d (and associated 2D extras) unless samples require them
• com.unity.visualscripting

This keeps the SDK lean; samples can declare extras via a Samples~ package or Setup Assistant.

If samples actually require these, keep them but scope to Samples~/sampleProject/manifest.json to avoid polluting consumer projects.

---

15-46: Scope core modules to the minimum needed.

For an HTTP/WebSocket SDK, builtin modules likely needed are: jsonserialize, unitywebrequest, unitywebrequesttexture, (optionally) imgui/ugui for samples. Others (audio, physics, terrain, etc.) can be omitted.

templates/unity/README.md.twig (2)

6-6: Swap Travis badge for GitHub Actions.

Project CI is on GitHub Actions per PR notes; the Travis badge is misleading.

-[![Build Status](https://img.shields.io/travis/com/appwrite/sdk-generator?style=flat-square)](https://travis-ci.com/appwrite/sdk-generator)
+[![CI](https://github.com/appwrite/sdk-generator/actions/workflows/tests.yml/badge.svg)](https://github.com/appwrite/sdk-generator/actions/workflows/tests.yml)

---

5-5: Make the Unity version badge configurable.

Hardcoding 2021.3+ is at odds with reviewer feedback to target newer LTS.

-![Unity](https://img.shields.io/badge/Unity-2021.3%2B-blue.svg)
+![Unity](https://img.shields.io/badge/Unity-{{ sdk.unityMinVersion | default('2022.3%2B') }}-blue.svg)

Set sdk.unityMinVersion in the language provider to align with the chosen LTS (2022.3+ or Unity 6).

tests/languages/unity/Tests.cs (3)

117-121: Avoid unsafe cast for Redirect() result

Guard the cast to prevent NullReferenceException in case the return type changes.

-            var result = await general.Redirect();
-            Debug.Log((result as Dictionary<string, object>)["result"]);
+            var result = await general.Redirect();
+            if (result is Dictionary<string, object> dict && dict.TryGetValue("result", out var res))
+                Debug.Log(res);
+            else
+                Assert.Fail("Unexpected redirect payload.");

---

255-258: Prefer Destroy over DestroyImmediate in PlayMode

In PlayMode tests, use Destroy to avoid editor-only semantics.

-                Object.DestroyImmediate(realtimeObject);
+                Object.Destroy(realtimeObject);

---

30-35: Surface original async exception

Throwing task.Exception rethrows AggregateException. Prefer GetAwaiter().GetResult() to preserve original stack for single-fault tasks.

-            if (task.Exception != null)
-            {
-                Debug.LogError($"Test failed with exception: {task.Exception}");
-                throw task.Exception;
-            }
+            if (task.IsFaulted)
+            {
+                Debug.LogError($"Test failed with exception: {task.Exception}");
+                task.GetAwaiter().GetResult(); // throws original
+            }

tests/Unity2021Test.php (1)

20-20: Fail fast when UNITY_LICENSE is missing; preserve exit codes through pipes

Currently, tests proceed even without a valid license. Add an explicit check and pipefail to avoid false greens.

-protected string $command = 'docker run --network="mockapi" --rm -v "$(pwd):/project" -w /project/tests/sdks/unity -e UNITY_LICENSE unityci/editor:ubuntu-2021.3.45f1-base-3.1.0 /bin/bash -c "echo \"\$UNITY_LICENSE\" > Unity_lic.ulf && /opt/unity/Editor/Unity -nographics -batchmode -manualLicenseFile Unity_lic.ulf -quit || true && /opt/unity/Editor/Unity -projectPath . -batchmode -nographics -runTests -testPlatform PlayMode -stackTraceLogType None -logFile - 2>/dev/null | sed -n \'/Test Started/,\$p\' | grep -v -E \'^(UnityEngine\\.|System\\.|Cysharp\\.|\\(Filename:|\\[.*\\]|##utp:|^\\s*\$|The header Origin is managed automatically|Connected to realtime:)\' | grep -v \'StackTraceUtility\'"';
+protected string $command = 'docker run --network="mockapi" --rm -v "$(pwd):/project" -w /project/tests/sdks/unity -e UNITY_LICENSE unityci/editor:ubuntu-2021.3.45f1-base-3.1.0 /bin/bash -lc "\
+set -euo pipefail; \
+if [ -z \"${UNITY_LICENSE:-}\" ]; then echo \"UNITY_LICENSE secret missing\" >&2; exit 1; fi; \
+printf \"%s\" \"\$UNITY_LICENSE\" > Unity_lic.ulf; \
+/opt/unity/Editor/Unity -nographics -batchmode -manualLicenseFile Unity_lic.ulf -quit || true; \
+/opt/unity/Editor/Unity -projectPath . -batchmode -nographics -runTests -testPlatform PlayMode -stackTraceLogType None -logFile - \
+  2>/dev/null | sed -n \'/Test Started/,\$p\' | grep -v -E \'^(UnityEngine\\.|System\\.|Cysharp\\.|\\(Filename:|\\[.*\\]|##utp:|^\\s*\$|The header Origin is managed automatically|Connected to realtime:)\' | grep -v \'StackTraceUtility\'"';

templates/unity/Assets/Runtime/Core/Plugins/WebGLCookies.jslib (1)

7-37: Consider more granular error handling in EnableWebGLHttpCredentials

While the function has a try-catch block, the empty catch could mask legitimate errors that developers should know about during debugging.

Consider logging errors in development builds:

-    } catch (e) { /* noop */ }
+    } catch (e) { 
+      if (typeof console !== 'undefined' && console.error) {
+        console.error('[Appwrite] Failed to enable WebGL HTTP credentials:', e);
+      }
+    }

src/SDK/Language/Unity.php (1)

390-405: Test mode filtering logic could be more maintainable

The test mode filtering uses string matching on template destinations, which could break if template paths change. Consider using a more robust approach.

Consider defining test exclusions by scope or adding a test-specific property to file definitions:

// Alternative approach: Add 'excludeInTest' property to file definitions
$files = [
    [
        'scope'         => 'default',
        'destination'   => 'Assets/Runtime/Utilities/{{ spec.title | caseUcfirst }}Utilities.cs',
        'template'      => 'unity/Assets/Runtime/Utilities/AppwriteUtilities.cs.twig',
        'excludeInTest' => true,  // Add this property
    ],
    // ... other files
];

// Then filter more simply:
if (isset($GLOBALS['UNITY_TEST_MODE']) && $GLOBALS['UNITY_TEST_MODE'] === true) {
    $files = array_filter($files, function ($file): bool {
        return !($file['excludeInTest'] ?? false);
    });
}

templates/unity/Assets/Samples~/AppwriteExample/AppwriteExample.cs.twig (1)

17-19: Fix formatting: Remove extra blank line

There's an unnecessary blank line between the method signature and opening brace.

         private async void Start()
-        
         {

templates/unity/Assets/Runtime/Utilities/AppwriteUtilities.cs.twig (1)

35-36: Remove unused variable assignment

Line 36 assigns manager.Realtime to variable a which is never used. This appears to be leftover debug/test code.

-            //Create Realtime instance
-            var a =manager.Realtime;

templates/unity/Assets/Runtime/Realtime.cs.twig (1)

358-383: Async void pattern in UniTask.Create

Using UniTask.Create with an async lambda creates a fire-and-forget task. Consider storing the task reference for proper lifecycle management.

Consider storing the heartbeat task:

private UniTask _heartbeatTask;

private void StartHeartbeat()
{
    StopHeartbeat();
    _heartbeatTokenSource = new CancellationTokenSource();
    
    _heartbeatTask = UniTask.Create(async () =>
    {
        // ... existing heartbeat logic ...
    });
}

templates/unity/Assets/Editor/AppwriteSetupWindow.cs.twig (1)

250-251: Potential race condition in delayed message clearing

Using Task.Delay with TaskScheduler.FromCurrentSynchronizationContext() for clearing messages could fail if the synchronization context changes or becomes unavailable.

Consider using Unity's coroutine system or EditorApplication callbacks:

private void ShowMessage(string message, MessageType type)
{
    _statusMessage = message;
    _statusMessageType = type;
    Repaint();
    
    if (type != MessageType.Error)
    {
        EditorApplication.delayCall += () =>
        {
            EditorCoroutineUtility.StartCoroutine(ClearMessageAfterDelay(message, 5f), this);
        };
    }
}

private System.Collections.IEnumerator ClearMessageAfterDelay(string message, float delay)
{
    yield return new EditorWaitForSeconds(delay);
    if (_statusMessage == message)
    {
        _statusMessage = "";
        Repaint();
    }
}

templates/unity/Assets/Runtime/AppwriteManager.cs.twig (1)

136-137: Hardcoded assumption about service namespace

The code assumes all services are in the same namespace as the Account class. This assumption should be documented or made more flexible.

Consider adding a comment to document this assumption:

+    // IMPORTANT: This assumes all service classes are in the same namespace as Account.
+    // If services are moved to different namespaces, this logic will need to be updated.
     var serviceNamespace = typeof(Account).Namespace; // Assumes all services are in the same namespace.

templates/unity/Assets/Runtime/AppwriteConfig.cs.twig (1)

38-41: Consider HTTPS default for cloud endpoints.

Using HTTPS by default aligns with security best practices. The realtime endpoint correctly uses wss:// for secure WebSocket connections.

-        [SerializeField] private string endpoint = "https://cloud.{{ spec.title | lower }}.io/v1";
+        [SerializeField] private string endpoint = "https://cloud.{{ spec.title | lower }}.io/v1";
         
         [Tooltip("WebSocket endpoint for realtime updates (optional)")]
         [SerializeField] private string realtimeEndpoint = "wss://cloud.{{ spec.title | lower }}.io/v1";

templates/unity/Assets/Editor/AppwriteSetupAssistant.cs.twig (2)

144-152: Consider adding timeout handling for package installation.

While the current implementation is functional, long-running package installations could benefit from timeout handling to prevent indefinite waiting.

Consider adding a timeout mechanism:

private static void InstallPackage(string packageUrl, Action onCompleted, float timeout = 60f)
{
    if (_isBusy) return;
    
    var queue = new Queue<string>();
    queue.Enqueue(packageUrl);
    
    var startTime = EditorApplication.timeSinceStartup;
    InstallNextPackage(queue, () => onCompleted?.Invoke(), (error) => {
        if (EditorApplication.timeSinceStartup - startTime > timeout) {
            Debug.LogError($"Package installation timed out after {timeout} seconds");
        }
        Debug.LogError(error);
    });
}

---

13-16: Verified — package URLs are valid and actively maintained.
Both UniTask and NativeWebSocket UPM Git URLs are documented in their upstream READMEs and show recent activity; no immediate change required. Optional: document a fallback (pinned version or alternate registry) for resilience.

templates/unity/Assets/Runtime/Core/Client.cs.twig (1)

725-731: Consider limiting certificate acceptance scope.

While accepting all certificates works for self-signed scenarios, consider validating against a specific certificate fingerprint or CA for better security.

 public class AcceptAllCertificatesSignedWithASpecificKeyPublicKey : CertificateHandler
 {
+    // Consider adding constructor to accept specific certificate fingerprint
+    private readonly byte[] _allowedFingerprint;
+    
+    public AcceptAllCertificatesSignedWithASpecificKeyPublicKey(byte[] allowedFingerprint = null)
+    {
+        _allowedFingerprint = allowedFingerprint;
+    }
+    
     protected override bool ValidateCertificate(byte[] certificateData)
     {
-        return true; // Accept all certificates
+        if (_allowedFingerprint == null) return true; // Accept all if no fingerprint specified
+        
+        // Validate against specific fingerprint for better security
+        // Implementation would compute hash of certificateData and compare
+        return true; // Simplified for now
     }
 }

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

example.php (1)

98-101: Sync response-format header with selected spec version.
Hardcoding '1.6.0' risks drift from $version ('1.8.x' above). Derive '1.8.0' from $version to keep this example accurate.

Apply this diff inside the Unity block:

-        ->setDefaultHeaders([
-            'X-Appwrite-Response-Format' => '1.6.0',
-        ])
+        ->setDefaultHeaders([
+            'X-Appwrite-Response-Format' => $responseFormat,
+        ])

And add this once after the $version assignment (outside this hunk):

// Derive concrete response format (e.g., '1.8.x' -> '1.8.0')
$responseFormat = preg_replace('/\.x$/', '.0', $version);

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

templates/unity/base/requests/oauth.twig (2)

9-12: Replace System.Web.HttpUtility; not available on Unity’s .NET Standard

HttpUtility.ParseQueryString will not compile on Unity 2021/2022/6. Use a small parser instead.

Apply:

-            var query = HttpUtility.ParseQueryString(callbackUri.Query);
-            var secret = query.Get("secret");
-            var key = query.Get("key");
-            var callbackDomain = query.Get("domain"); // Get domain from callback
+            var pairs = callbackUri.Query.TrimStart('?').Split('&', StringSplitOptions.RemoveEmptyEntries);
+            var dict = new Dictionary<string, string>(StringComparer.OrdinalIgnoreCase);
+            foreach (var p in pairs)
+            {
+                var kv = p.Split('=', 2);
+                var k = Uri.UnescapeDataString(kv[0]);
+                var v = kv.Length > 1 ? Uri.UnescapeDataString(kv[1]) : string.Empty;
+                dict[k] = v;
+            }
+            dict.TryGetValue("secret", out var secret);
+            dict.TryGetValue("key", out var key);
+            dict.TryGetValue("domain", out var callbackDomain); // optional override

Note: ensure using System; using System.Collections.Generic; are present.

---

25-31: Gate cookie write in Editor to match the warning

Currently cookies are parsed even in Editor, contradicting the warning. Skip cookie creation in Editor.

Apply:

-            var setCookieHeader = $"{key}={secret}; Path=/; Domain={parsedDomain}; Secure; HttpOnly; Max-Age={30 * 24 * 60 * 60}";
-            Debug.Log($"Setting session cookie for domain: {parsedDomain}");
-            _client.CookieContainer.ParseSetCookieHeader(setCookieHeader, parsedDomain);
-
-#if UNITY_EDITOR
-            Debug.LogWarning("[Appwrite] OAuth authorization in Editor: you can open and authorize, but cookies cannot be obtained. The session will not be set.");
-#endif
+            #if UNITY_EDITOR
+            Debug.LogWarning("[Appwrite] OAuth in Editor: cookies are not persisted; session will not be set.");
+            #else
+            var setCookieHeader = $"{key}={secret}; Path=/; Domain={parsedDomain}; Secure; HttpOnly; Max-Age={30 * 24 * 60 * 60}";
+            Debug.Log($"Setting session cookie for domain: {parsedDomain}");
+            _client.CookieContainer.ParseSetCookieHeader(setCookieHeader, parsedDomain);
+            #endif

templates/unity/Assets/Runtime/Realtime.cs.twig (3)

292-307: Null-guard _client/Config in fallback auth

Avoid potential NREs if Initialize hasn’t run or Config is null.

Apply:

         private void SendFallbackAuthentication()
         {
-            var session = _client.Config.GetValueOrDefault("session");
+            if (_client?.Config == null)
+            {
+                Debug.LogError("[Realtime] Cannot send fallback authentication: client/config is null");
+                return;
+            }
+            var session = _client.Config.GetValueOrDefault("session");

---

463-466: Avoid async cleanup in OnDestroy; perform best-effort synchronous teardown

Unity doesn’t await async work on destroy. Stop heartbeat/reconnect and close the socket immediately.

Apply:

-        private void OnDestroy()
-        {
-            Disconnect().Forget();
-        }
+        private void OnDestroy()
+        {
+            _reconnect = false;
+            StopHeartbeat();
+            _cancellationTokenSource?.Cancel();
+            if (_webSocket != null && _webSocket.State != WebSocketState.Closed)
+            {
+                // fire-and-forget close; Unity won't await here
+                _webSocket.Close();
+            }
+        }

---

126-152: Channels added during in-flight connect can be missed; also reset reconnect flag

If Subscribe is called while a connection is being established, the second CreateSocket() returns early due to _creatingSocket, and the new channels are not included after the first connect completes. Also, after a previous CloseConnection(), _reconnect stays false; set it before connecting.

Apply:

             var subscription = new RealtimeSubscription
             {
                 Channels = channels,
                 OnMessage = callback,
                 OnClose = () => CloseSubscription(subscriptionId, channels)
             };
@@
-            CreateSocket().Forget();
+            _reconnect = true;
+            if (_creatingSocket)
+            {
+                UniTask.Create(async () =>
+                {
+                    await UniTask.WaitUntil(() => !_creatingSocket);
+                    await CreateSocket();
+                });
+            }
+            else
+            {
+                UniTask.Create(async () => { await CreateSocket(); });
+            }

templates/unity/Assets/Runtime/Realtime.cs.twig (1)

100-101: Unused cancellation token source

_cancellationTokenSource is declared and cancelled but never created or used to drive operations. Either wire it into connect/heartbeat flows or remove it.

Also applies to: 448-456

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro